Skip to content

refactor(file): derive extraction format extensions from display#10412

Merged
jdx merged 1 commit into
jdx:mainfrom
risu729:refactor/extraction-format-display-extension
Jun 13, 2026
Merged

refactor(file): derive extraction format extensions from display#10412
jdx merged 1 commit into
jdx:mainfrom
risu729:refactor/extraction-format-display-extension

Conversation

@risu729

@risu729 risu729 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • derive ExtractionFormat::extension() from the enum display string for non-raw formats
  • pin Zip display to zip while preserving vsix parsing
  • simplify HTTP extension handling now that extension() returns an owned string

Conflict check

Tests

  • cargo fmt
  • cargo test test_extraction_format_extension_uses_canonical_display
  • cargo test test_extraction_format_from_file_name
  • cargo check

Summary by CodeRabbit

  • Bug Fixes

    • Corrected and standardized file format display strings for consistency.
    • Improved extension handling logic for file operations.
  • Tests

    • Added tests for file extension handling.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 96ac2a17-5dee-4079-854e-ba9eded38942

📥 Commits

Reviewing files that changed from the base of the PR and between 956765a and e12af9d.

📒 Files selected for processing (2)
  • src/backend/http.rs
  • src/file.rs

📝 Walkthrough

Walkthrough

This PR refactors ExtractionFormat's string representation system by standardizing its strum attributes and changing the extension() method to derive canonical strings via to_string() instead of a static match. The consumer in FileInfo::new is simplified to use the updated method signature.

Changes

ExtractionFormat Extension Refactor

Layer / File(s) Summary
ExtractionFormat string mappings and extension method
src/file.rs
ExtractionFormat enum gains corrected/standardized strum attributes for archive/compressed variants; extension() now returns Option<String> computed from self.to_string() for non-Raw formats (returning None for Raw). New test validates canonical display strings and None return for Raw.
FileInfo extension field simplification
src/backend/http.rs
FileInfo::new removes intermediate map() and simplifies extension field derivation using the refactored extension() method with unwrap_or_else() fallback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • jdx/mise#10403: Adds new tbz strum alias for TarBz2 alongside the string/extension mapping logic updates in ExtractionFormat.
  • jdx/mise#10409: Modifies ExtractionFormat::extension() behavior and to_string()/serialization mapping in src/file.rs, affecting the same method signature and string derivation path.

Poem

🐰 Archive formats unite, their strings now unified and bright,
From tar to zip, each extension flows, the display mapping glows,
FileInfo reads the path with ease, extension refactoring brings such peace!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring: deriving extraction format extensions from their display representations, which aligns with the core change of updating ExtractionFormat::extension() behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors ExtractionFormat::extension() to derive its return value from the enum's Display implementation rather than maintaining a parallel match arm per variant. To make that safe, explicit to_string annotations are added to every multi-alias variant, pinning the canonical display string independently of serialization order.

  • extension() is simplified from a 19-arm match to a single .then(|| self.to_string()) expression, and its return type changes from Option<&'static str> to Option<String>.
  • Zip gains an explicit to_string = "zip" to prevent the longer vsix alias from ever being selected as Display by strum.
  • A new test verifies that to_string on the key tar variants and Zip returns the expected canonical extension, and that Raw returns None.

Confidence Score: 5/5

Safe to merge — pure refactor with no behavior changes; all callers of extension() are updated and the new logic is verified by an existing + new test.

The change replaces a hand-maintained 19-arm match with a one-liner backed by strum's Display, which is already tested by the new test_extraction_format_extension_uses_canonical_display test. The only API-surface change (return type &'static strString) has exactly one other call site (http.rs), which is correctly updated in the same PR. No logic is altered — every variant that previously returned Some("…") still returns the same string, and Raw still returns None.

No files require special attention.

Important Files Changed

Filename Overview
src/file.rs Pins to_string on multi-alias variants, simplifies extension() to derive from Display, and adds a targeted test for the canonical extensions; logic is correct and behavior-preserving.
src/backend/http.rs Removes the now-redundant `.map(

Reviews (2): Last reviewed commit: "refactor(file): derive extraction format..." | Re-trigger Greptile

Comment thread src/file.rs
@risu729 risu729 force-pushed the refactor/extraction-format-display-extension branch from 49910ad to e12af9d Compare June 13, 2026 21:21
@risu729 risu729 marked this pull request as ready for review June 13, 2026 21:24
@jdx jdx enabled auto-merge (squash) June 13, 2026 21:32
@jdx jdx merged commit 6858a3e into jdx:main Jun 13, 2026
33 checks passed
@risu729 risu729 deleted the refactor/extraction-format-display-extension branch June 13, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants